-
Notifications
You must be signed in to change notification settings - Fork 269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Start working on main ctapipe cli tool #2371
Conversation
I remember trying this in the past and there was something in our Tool base class that broke sub-commands. I guess whatever it was is fixed! Nice |
At least I didn't find anything immediately wrong:
for example works correctly here.
|
"ctapipe.tools.train_disp_regressor.TrainDispRegressor", | ||
"Train telescope-type-wise disp regression models for direction reconstruction", | ||
), | ||
"info": ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing "fileinfo"
tool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed a few commits to convert ctapipe-fileinfo
into a Tool
and include it in this list. So this should be fixed now.
If you want to go full-hierarchy, you could also add a second layer for
Especially since we may add |
- uses inspect.cleandoc() to properly strip away whitespace - reformat a few to avoid lines longer than 80 cols
A few things still missing here:
|
running `ctapipe-info --all` gave no results, as the run functino was overwritten Also removed provenance output, as not needed for this tool
prevented ctapipe-info --tools from displaying a message
One more issue to solve: commands that use positional arguments don't seem to work in sub-commands. For example: Works:
Fails:
It still works using
but that defeats the purpose and makes it hard to do thing like This may also affect the merge tool, which also accepts positional args |
Fixes #850
I will also update the tools documentation in this PR, leaving as draft for now